feat: add remote MCP readiness - #22
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb9bdf694
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| return { | ||
| parsedUrl, | ||
| sanitizedUrl: `${parsedUrl.origin}${parsedUrl.pathname}`, |
There was a problem hiding this comment.
Redact payloads from unsupported URL schemes
When a configured URL uses an unsupported but parseable scheme such as data:text/plain,token or file:///private/path, origin is "null" and pathname retains the entire opaque payload or local path. This sanitizedUrl is subsequently included in validation findings, security reports, and runtime plans, so secrets or private paths can leak into CI output; return null or a fixed redacted value for non-HTTP(S) schemes.
Useful? React with 👍 / 👎.
| ...(sessionId === null ? {} : { "MCP-Session-Id": sessionId }) | ||
| } | ||
| }); | ||
| if (initializedResponse.statusCode < 200 || initializedResponse.statusCode >= 300) { |
There was a problem hiding this comment.
Require 202 for initialized notifications
When an endpoint responds to notifications/initialized with another 2xx status such as 200 or 204, this condition marks protocolHeaders as passing. Streamable HTTP requires an accepted JSON-RPC notification to receive HTTP 202 with no response body, so the readiness probe currently reports nonconforming servers as healthy; require status 202 here.
Useful? React with 👍 / 👎.
| return inspection.parsedUrl.protocol === "http:" ? "loopback_http" : "loopback_https"; | ||
| } | ||
|
|
||
| return "public_https"; |
There was a problem hiding this comment.
Distinguish public HTTP in runtime plans
For every successfully parsed non-loopback endpoint, including http://example.com/mcp, this function returns public_https. The resulting review plan therefore misstates that a plaintext public transport is HTTPS, which can mislead reviewers or automation consuming networkClass even though a separate security finding makes the plan fail; represent public HTTP separately or classify it as invalid.
Useful? React with 👍 / 👎.
Adds opt-in Remote MCP Streamable HTTP initialization checks, OAuth metadata readiness, SSRF-resistant bounded networking, approval/reporting integration, GitHub Action controls, public security documentation, and dependency audit remediation. Verified with the authoritative release-check, 811 tests, build, package dry-run, clean consumer install, and zero npm audit findings.